Fix #3405 Fix incorrect reuse of code mining annotations with mismatched types #3416

travkin79 · 2025-10-20T15:24:15Z

There are three types of code minings and three corresponding types of annotations. Issue #3405 describes a problem with updating the annotations. In certain cases the annotations are not re-created if the code mining type changes, e.g. from in-line code minings to line header code minings. This PR fixes that issue.

In order to make reviews easy, I left three commits for three different changes. The commits can be squashed if you prefer having only one commit.

tobiasmelcher

Thanks a lot for the fixes, great work.
Could you please also create a unit test which documents the problem scenario where the CodeMiningType for a given offset is changed?

...org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningManager.java

...lipse.jface.text.examples/src/org/eclipse/jface/text/examples/codemining/CodeMiningDemo.java

travkin79 · 2025-10-20T16:01:48Z

Could you please also create a unit test which documents the problem scenario where the CodeMiningType for a given offset is changed?

Hi @tobiasmelcher,
Thank you for the feedback. I'll incorporate corresponding changes.

I was thinking about how to implement a test for my case, but for some reason, some tests, esp. CodeMining test, fail on my machine on the master branch (without my changes), commit hash 0ce1430. I didn't find the reason yet.

github-actions · 2025-10-20T17:30:23Z

Test Results

3 018 files ±0 3 018 suites ±0 2h 33m 6s ⏱️ + 13m 51s
8 234 tests +1 7 984 ✅ ±0 249 💤 ±0 1 ❌ +1
23 622 runs +3 22 827 ✅ +2 794 💤 ±0 1 ❌ +1

For more details on these failures, see this check.

Results for commit 673bde4. ± Comparison against base commit ce17c9e.

♻️ This comment has been updated with latest results.

travkin79 · 2025-10-21T14:57:28Z

Hi @tobiasmelcher and @BeckerWdf,

I'm trying to implement a test for issue #3405, but I'm struggling with various issues with running the tests on my machine(s). I might need some help.

Some tests fail for the unmodified Eclipse platform UI master branch, commit 0ce1430, on MacOS and on Windows 11 as well. Due to the failing tests, I cannot assure that my new test will behave as expected.

In addition, the SWT widgets or the tests seem to behave strange. For example, when debugging my new test case on MacOS, the first line's vertical line indentation is reported to be 16, but the code mining is being drawn onto the source viewer's content instead of drawing it into an extra line (see screenshot 1). The in-line code mining is sometimes also drawn into the source viewer's text (see screeshot 2). The method getStyleRangeAtOffset(...) seems to always return null, on both operating systems (Mac & Win). Thus, I don't know which conditions to choose in my tests in order to check if the code minings are correctly drawn.

Screenshot 1:

Screenshot 2:

I've extended the code mining demo where the code minings seem to behave as expected, i.e. drawing a "title" either above a reference (e.g. "REF-X" in my example) in a line header or in the same line in front of the referencing text:

Line Header:

In-line:

It would be great if someone could take a look at my PR draft and help me fixing the tests.

tobiasmelcher · 2025-10-21T15:08:50Z

@travkin79 sure, I'll take a look and make a proposal. Please give me some time. I will also check if I can reproduce the other test errors you have mentioned.

travkin79 · 2025-10-21T15:58:29Z

Thank you @tobiasmelcher,

I managed to finish a first version of a test. It turns out, getStyleRangeAtOffset(...) does return something != null, but for an unexpected index. For a text "Here REF-X..." where a code mining label is placed after "Here ", I expected getStyleRangeAtOffset(5) to be non-null, but instead I get a non-null result for getStyleRangeAtOffset(4), i.e. at the position of the space character. That looks wrong to me.

Besides that, there might be an issue with my PR in case that someone is using AbstractCodeMining class instead of sub-classing one of LineContentCodeMining, LineHeaderCodeMining, or DocumentFooterCodeMining. If none of the latter classes is sub-classed, we cannot determine which of the three corresponding code mining modes we have. I saw a few tests fail because of using anonymous sub-classes of AbstractCodeMining and fixed these, but there might be some client code doing something similar. I guess, moving AbstractCodeMining to an internal package is not an option.

iloveeclipse

@travkin79 : Please

squash all the changes from 6 commits to one or two.
Provide meaningful commit message, ideally following https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#commit-message-recommendations
featch latest master state and rebase your branch on it
Force push so PR is updated

...org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningManager.java

tobiasmelcher · 2025-10-24T07:54:48Z

there might be an issue with my PR in case that someone is

The tests of CodeMiningTest also fail on windows on my machine.

I assume the test errors appear if you have monitor scaling factory larger than 1.0.
I will take care of this issue.

travkin79 · 2025-10-24T08:34:40Z

@travkin79 : Please

squash all the changes from 6 commits to one or two.

Provide meaningful commit message, ideally following https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#commit-message-recommendations

featch latest master state and rebase your branch on it

Force push so PR is updated

Done. Thank you for looking at my PR

travkin79 · 2025-10-24T08:36:29Z

Hi @tobiasmelcher,

I assume the test errors appear if you have monitor scaling factory larger than 1.0.
I will take care of this issue.

I think, fixing these tests should be done in a separate issue / PR. I just wanted to be sure, that I'm not doing something wrong when running the existing tests.

In addition to the tests already mentioned, the following tests do also fail on my machine. I guess, it might be for the same reason.

tobiasmelcher · 2025-10-24T11:28:33Z

Thank you @tobiasmelcher,

I managed to finish a first version of a test. It turns out, getStyleRangeAtOffset(...) does return something != null, but for an unexpected index. For a text "Here REF-X..." where a code mining label is placed after "Here ", I expected getStyleRangeAtOffset(5) to be non-null, but instead I get a non-null result for getStyleRangeAtOffset(4), i.e. at the position of the space character. That looks wrong to me.

@travkin79 : If you want to work with offset 5 inside the unit test implementation, you need to set afterPosition of LineContentCodeMining to true. Then there is no - 1 necessary.

private static class ReferenceInLineCodeMining extends LineContentCodeMining {

		public ReferenceInLineCodeMining(String label, int positionOffset, ICodeMiningProvider provider) {
			super(new Position(positionOffset, 1), true, provider, null); // <!----- second argument needs to be set to true (default is false)
			this.setLabel(label);
		}

	}

tobiasmelcher · 2025-10-24T11:30:04Z

just wanted to be sure, that I'm not doing something wrong when running the existing tests.

In addition to the tests already mentione

Sure, the tests will be fixed via a separate pull request.

tobiasmelcher · 2025-10-24T12:04:49Z

Besides that, there might be an issue with my PR in case that someone is using AbstractCodeMining class instead of sub-classing one of LineContentCodeMining, LineHeaderCodeMining, or DocumentFooterCodeMining. If none of the latter classes is sub-classed, we cannot determine which of the three corresponding code mining modes we have. I saw a few tests fail because of using anonymous sub-classes of AbstractCodeMining and fixed these, but there might be some client code doing something similar. I guess, moving AbstractCodeMining to an internal package is not an option.

@travkin79 : This is an important topic. We need to support the case that implementations return only an instance of AbstractCodeMining. The old implementation then used CodeMiningLineContentAnnotation. Instead of raising an IllegalStateException inside CodeMiningMode#createFor we need to set the mode to InLine. Could you please update the code? Thanks a lot.

travkin79 · 2025-10-24T12:38:40Z

Besides that, there might be an issue with my PR in case that someone is using AbstractCodeMining class instead of sub-classing one of LineContentCodeMining, LineHeaderCodeMining, or DocumentFooterCodeMining. If none of the latter classes is sub-classed, we cannot determine which of the three corresponding code mining modes we have. I saw a few tests fail because of using anonymous sub-classes of AbstractCodeMining and fixed these, but there might be some client code doing something similar. I guess, moving AbstractCodeMining to an internal package is not an option.

@travkin79 : This is an important topic. We need to support the case that implementations return only an instance of AbstractCodeMining. The old implementation then used CodeMiningLineContentAnnotation. Instead of raising an IllegalStateException inside CodeMiningMode#createFor we need to set the mode to InLine. Could you please update the code? Thanks a lot.

Hi @tobiasmelcher,
I did exactly that before, but faced another problem. I tested my implementation (without throwing an exception) with the new Code Mining Demo example and with the existing code mining tests. In some cases using CodeMiningLineContentAnnotation as a default lead to the source viewer being re-painted over and over again and the scroll bar was flickering (getting longer and then shorter over and over again). Thus, I think, we need to check that case in more detail.

I could create a separate git branch, where you could reproduce and debug that case on your machine if you like.

tobiasmelcher · 2025-10-24T12:48:41Z

Besides that, there might be an issue with my PR in case that someone is using AbstractCodeMining class instead of sub-classing one of LineContentCodeMining, LineHeaderCodeMining, or DocumentFooterCodeMining. If none of the latter classes is sub-classed, we cannot determine which of the three corresponding code mining modes we have. I saw a few tests fail because of using anonymous sub-classes of AbstractCodeMining and fixed these, but there might be some client code doing something similar. I guess, moving AbstractCodeMining to an internal package is not an option.

@travkin79 : This is an important topic. We need to support the case that implementations return only an instance of AbstractCodeMining. The old implementation then used CodeMiningLineContentAnnotation. Instead of raising an IllegalStateException inside CodeMiningMode#createFor we need to set the mode to InLine. Could you please update the code? Thanks a lot.

Hi @tobiasmelcher, I did exactly that before, but faced another problem. I tested my implementation (without throwing an exception) with the new Code Mining Demo example and with the existing code mining tests. In some cases using CodeMiningLineContentAnnotation as a default lead to the source viewer being re-painted over and over again and the scroll bar was flickering (getting longer and then shorter over and over again). Thus, I think, we need to check that case in more detail.

I could create a separate git branch, where you could reproduce and debug that case on your machine if you like.

That would be very helpful, thanks. Could you please also tell me then the steps how to reproduce the problem scenario?

travkin79 · 2025-10-24T13:11:56Z

Thank you @tobiasmelcher,
I managed to finish a first version of a test. It turns out, getStyleRangeAtOffset(...) does return something != null, but for an unexpected index. For a text "Here REF-X..." where a code mining label is placed after "Here ", I expected getStyleRangeAtOffset(5) to be non-null, but instead I get a non-null result for getStyleRangeAtOffset(4), i.e. at the position of the space character. That looks wrong to me.

@travkin79 : If you want to work with offset 5 inside the unit test implementation, you need to set afterPosition of LineContentCodeMining to true. Then there is no - 1 necessary.
private static class ReferenceInLineCodeMining extends LineContentCodeMining {

		public ReferenceInLineCodeMining(String label, int positionOffset, ICodeMiningProvider provider) {
			super(new Position(positionOffset, 1), true, provider, null); // <!----- second argument needs to be set to true (default is false)
			this.setLabel(label);
		}

	}

Hi @tobiasmelcher,
Thank you for the hint. I adapted my test and code mining demo code (for now in a separate commit that can be squashed later).

But checking the implementation in CodeMiningManager I looks to me as if afterPosition should always be true for CodeMiningLineContentAnnotation and LineContentCodeMining. If I am write, then the constructors should be adapted accordingly.

tobiasmelcher · 2025-10-24T13:22:52Z

Thank you @tobiasmelcher,
I managed to finish a first version of a test. It turns out, getStyleRangeAtOffset(...) does return something != null, but for an unexpected index. For a text "Here REF-X..." where a code mining label is placed after "Here ", I expected getStyleRangeAtOffset(5) to be non-null, but instead I get a non-null result for getStyleRangeAtOffset(4), i.e. at the position of the space character. That looks wrong to me.

@travkin79 : If you want to work with offset 5 inside the unit test implementation, you need to set afterPosition of LineContentCodeMining to true. Then there is no - 1 necessary.
private static class ReferenceInLineCodeMining extends LineContentCodeMining {

		public ReferenceInLineCodeMining(String label, int positionOffset, ICodeMiningProvider provider) {
			super(new Position(positionOffset, 1), true, provider, null); // <!----- second argument needs to be set to true (default is false)
			this.setLabel(label);
		}

	}
Hi @tobiasmelcher, Thank you for the hint. I adapted my test and code mining demo code (for now in a separate commit that can be squashed later).

But checking the implementation in CodeMiningManager I looks to me as if afterPosition should always be true for CodeMiningLineContentAnnotation and LineContentCodeMining. If I am write, then the constructors should be adapted accordingly.

That's a compatibility issue. We cannot change the default. Code minings were initially implemented to support the parameter hints where the mining should be drawn before a given parameter.

I introduced at a later point in time the afterPosition argument in the Code Mining API which had to be introduced in a compatible way. The afterPosition mechanism is used to realize AI based code completions (the Copilot plugin is one user of this argument).

travkin79 · 2025-10-24T13:36:15Z

Thank you for the explanation @tobiasmelcher,
I created a git branch for testing purposes, where I no longer throw an exception if the CodeMiningManager doesn't find one of the three expected sub-types of AbstractCodeMining. I also set the default code mining mode back to line header, since that was originally the case and this way we can reproduce the permanent redrawing issue. In addition, I reverted replacing AbstractCodeMining with LineContentCodeMining in some tests and examples.

Surprisingly, setting the default to CodeMiningMode.InLine in CodeMiningMode#createFor does not produce the flickering scroll bar / redrawing code minings issue. But I'm not sure if the issue only disappears for our code mining demo example. It could still occur in other cases.

git branch: https://github.com/travkin79/eclipse.platform.ui/tree/test-code-minings

Steps to reproduce
Run the org.eclipse.jface.text.examples.codemining.CodeMiningDemo as a Java application (Run as...)

Result
You'll see a window with some code where the scroll bar is flickering, becoming bigger and smaller again, over and over again. (see attached 5 sec video)

code-mining-permanent-redrawing-issue.mp4

tobiasmelcher · 2025-10-27T09:56:17Z

CodeMiningDemo

Hi @travkin79 ,
thanks a lot for providing all the details, that helped a lot. I was able to reproduce the flickering scenario.
I compared the old decision logic what kind of annotation should be created boolean inLineHeader= !minings.isEmpty() ? (first instanceof LineHeaderCodeMining) : true; with the new one CodeMiningMode#createFor and found a small difference. If minings list is not empty and first entry is not a LineHeaderCodeMining then a CodeMiningLineContentAnnotation was created in the old logic. This is not the case for the new implementation.

I changed CodeMiningMode#createFor the following way and flickering no longer occurs.

public static CodeMiningMode createFor(List<ICodeMining> minings) {
	Assert.isNotNull(minings);

	CodeMiningMode mode= CodeMiningMode.HeaderLine;
	if (!minings.isEmpty()) {
		ICodeMining first= minings.get(0);

		if (CodeMiningMode.InLine.codeMiningType.isInstance(first)) {
			mode= CodeMiningMode.InLine;
		} else if (CodeMiningMode.HeaderLine.codeMiningType.isInstance(first)) {
			mode= CodeMiningMode.HeaderLine;
		} else if (CodeMiningMode.FooterLine.codeMiningType.isInstance(first)) {
			mode= CodeMiningMode.FooterLine;
		} else {
			mode= CodeMiningMode.InLine; // <!------------ add this else branch
		}
	}
	return mode;
}

Can we go with this approach? I did not further analyze the flickering issue and cannot explain what happens. I think we are safe if we can ensure that the new implementation creates the same annotations as before.

With best regards,
Tobias

travkin79 · 2025-10-27T14:19:32Z

Hi @tobiasmelcher,
Now I see the case that I didn't consider. Thank you very much for analyzing. Yes, I agree with that solution. If there are no objections, we can go with this solution. Can you add your fix while merging?

Best regards,
Dietrich

tobiasmelcher · 2025-10-27T14:59:58Z

Hi @tobiasmelcher, Now I see the case that I didn't consider. Thank you very much for analyzing. Yes, I agree with that solution. If there are no objections, we can go with this solution. Can you add your fix while merging?

Best regards, Dietrich

I hope I updated the pull request the correct way? If everything is fine, we need to squash the changes. I can do it if you want.

travkin79 · 2025-10-27T15:07:55Z

Hi @tobiasmelcher,
It would be great if you could squash the commits and rebase on master.

Drawing code minings depends on the type of code mining annotations that are created for code minings (that are created by code mining providers). When redrawing code minings, the corresponding annotations have to be updated or re-created. The annotations' types have to comply with the code minings' types, i.e. a LineContentCodeMining must go with a CodeMiningLineContentAnnotation, a LineHeaderCodeMining with a CodeMiningLineHeaderAnnotation, and a DocumentFooterCodeMining with a CodeMiningDocumentFooterAnnotation. This PR fixes re-creation of annotations. In some cases, annotations were reused, despite of their wrong type. Now, they are re-created with the required type. The PR also extends the code mining demo to reproduce the issue eclipse-platform#3405 and adds an automated test. Fixes eclipse-platform#3405

tobiasmelcher · 2025-10-27T17:16:07Z

Test error in LinkHelperTest.testLinkHelperEditorActivation not related to this change

BeckerWdf · 2025-10-28T07:21:54Z

So is this good to be merged now?

tobiasmelcher · 2025-10-28T09:11:26Z

So is this good to be merged now?

Yes, from my point of view this pull request is ready to be merged.

BeckerWdf · 2025-10-28T12:08:12Z

@travkin79: Thanks for providing a PR.

travkin79 · 2025-10-28T12:37:23Z

Thank you all for the reviews, analysis, and merging. 👍

BeckerWdf requested a review from tobiasmelcher October 20, 2025 15:28

tobiasmelcher reviewed Oct 20, 2025

View reviewed changes

travkin79 force-pushed the feature/patch-3405-code-minings branch 2 times, most recently from 8b1e2fd to e85cb9d Compare October 21, 2025 15:27

travkin79 marked this pull request as ready for review October 21, 2025 15:59

iloveeclipse reviewed Oct 24, 2025

View reviewed changes

...org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningManager.java Show resolved Hide resolved

travkin79 force-pushed the feature/patch-3405-code-minings branch 2 times, most recently from 29e4efd to fe9f52e Compare October 24, 2025 08:31

tobiasmelcher force-pushed the feature/patch-3405-code-minings branch from b271e0d to 673bde4 Compare October 27, 2025 16:00

tobiasmelcher changed the title ~~Fix #3405 Updating code minings~~ Fix #3405 Fix incorrect reuse of code mining annotations with mismatched types Oct 27, 2025

tobiasmelcher merged commit 1563627 into eclipse-platform:master Oct 28, 2025
16 of 18 checks passed

BeckerWdf added this to the 4.38 M3 milestone Oct 28, 2025

BeckerWdf assigned travkin79 Oct 28, 2025

iloveeclipse mentioned this pull request Oct 28, 2025

Code minings are not correctly redrawn when switching between in-line and line header mode #3405

Closed

Uh oh!

Fix #3405 Fix incorrect reuse of code mining annotations with mismatched types #3416

Fix #3405 Fix incorrect reuse of code mining annotations with mismatched types #3416

Conversation

travkin79 commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasmelcher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

travkin79 commented Oct 20, 2025

Uh oh!

github-actions bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

travkin79 commented Oct 21, 2025

Uh oh!

tobiasmelcher commented Oct 21, 2025

Uh oh!

travkin79 commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iloveeclipse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tobiasmelcher commented Oct 24, 2025

Uh oh!

travkin79 commented Oct 24, 2025

Uh oh!

travkin79 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasmelcher commented Oct 24, 2025

Uh oh!

tobiasmelcher commented Oct 24, 2025

Uh oh!

tobiasmelcher commented Oct 24, 2025

Uh oh!

travkin79 commented Oct 24, 2025

Uh oh!

tobiasmelcher commented Oct 24, 2025

Uh oh!

travkin79 commented Oct 24, 2025

Uh oh!

tobiasmelcher commented Oct 24, 2025

Uh oh!

travkin79 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasmelcher commented Oct 27, 2025

Uh oh!

travkin79 commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tobiasmelcher commented Oct 27, 2025

Uh oh!

travkin79 commented Oct 27, 2025

Uh oh!

tobiasmelcher commented Oct 27, 2025

Uh oh!

BeckerWdf commented Oct 28, 2025

Uh oh!

tobiasmelcher commented Oct 28, 2025

Uh oh!

Uh oh!

BeckerWdf commented Oct 28, 2025

Uh oh!

travkin79 commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

travkin79 commented Oct 20, 2025 •

edited

Loading

github-actions bot commented Oct 20, 2025 •

edited

Loading

travkin79 commented Oct 21, 2025 •

edited

Loading

travkin79 commented Oct 24, 2025 •

edited

Loading

travkin79 commented Oct 24, 2025 •

edited

Loading

travkin79 commented Oct 27, 2025 •

edited

Loading